Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow modify fsGroupPolicy for csidriver #4363

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Conversation

maximus13th
Copy link
Contributor

Describe what this PR does

Hello!
This PR allows to modify parameter fsGroupPolicy in cephfs-csi driver.
It is useful for closing bugs like this
https://bugzilla.redhat.com/show_bug.cgi?id=2059248
For example, I faced a problem when I tried to mount the existing CephFS volume to our application with the next security parameters

securityContext:
  fsGroup: 82
  runAsGroup: 82
  runAsUser: 82

Since there are more than 1 million files in this volume it tries to chown all of them, as I understand it, and the pods get stuck in the ContainerCreating state for a very long time.

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?
Yes.
Are there concerns around backward compatibility?
No.
Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic
Copy link
Member

nixpanic commented Jan 5, 2024

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Jan 5, 2024

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (2/2)
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git rebase --skip'
interactive rebase in progress; onto 0ec64b75
Last commands done (2 commands done):
   pick 2fdf65aa cephfs: allow modify fsGroupPolicy for csidriver
   pick 7cf32601 allow modify fsGroupPolicy for csidriver
No commands remaining.
You are currently rebasing branch 'devel' on '0ec64b75'.
  (all conflicts fixed: run "git rebase --continue")

nothing to commit, working tree clean
Could not apply 7cf32601... allow modify fsGroupPolicy for csidriver

@nixpanic
Copy link
Member

nixpanic commented Jan 5, 2024

Hi @maximus13th , I've cleaned up the commits in this PR for you. There is now a single commit that we can hopefully merge. Thanks for your contribution!

@nixpanic nixpanic added component/cephfs Issues related to CephFS component/deployment Helm chart, kubernetes templates and configuration Issues/PRs labels Jan 5, 2024
nixpanic
nixpanic previously approved these changes Jan 5, 2024
@nixpanic nixpanic requested a review from a team January 5, 2024 08:58
allow to change value of fsGroupPolicy parameter for CSI Driver spec

Signed-off-by: maximus13th <maxym.pariy@gmail.com>
@nixpanic nixpanic requested a review from a team January 5, 2024 10:55
@riya-singhal31
Copy link
Contributor

@Mergifyio queue

Copy link
Contributor

mergify bot commented Jan 8, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jan 8, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jan 8, 2024
@nixpanic nixpanic added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jan 8, 2024
@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

@maximus13th "ci/centos/k8s-e2e-external-storage/1.28" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

@maximus13th "ci/centos/k8s-e2e-external-storage/1.29" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

@maximus13th "ci/centos/mini-e2e-helm/k8s-1.27" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

Copy link
Contributor

mergify bot commented Jan 8, 2024

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot merged commit 51decb0 into ceph:devel Jan 8, 2024
42 checks passed
@maximus13th
Copy link
Contributor Author

Hello!
I see you merged this PR but these changes are absent in the recent release 3.10.2. When do you plan to add it to the release?

Best regards

@nixpanic
Copy link
Member

Hello! I see you merged this PR but these changes are absent in the recent release 3.10.2. When do you plan to add it to the release?

Fine by me. @Rakshith-R , if you agree too, can you add the backport label?

@Rakshith-R
Copy link
Contributor

I think we can back port it.
v3.10.2 was just released a few weeks ago.
and we just have two more backported commits.

So another new release may take quiet some time.

@Rakshith-R Rakshith-R added the backport-to-release-v3.10 Label to backport from devel to release-v3.10 branch label Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-release-v3.10 Label to backport from devel to release-v3.10 branch ci/retry/e2e Label to retry e2e retesting on approved PR's component/cephfs Issues related to CephFS component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants